From f5c9861db21091b3bdf19a14f2b09c0c17a0f3bd Mon Sep 17 00:00:00 2001 From: tsteven4 Date: Tue, 3 Jul 2018 11:40:57 -0600 Subject: [PATCH] cleanup filter class issues. (#215) --- GPSBabel.pro | 1 + .../filter_skeleton.cc | 0 filter.h | 13 ++++++++ route.cc | 2 +- trackfilter.cc | 32 +++++++------------ trackfilter.h | 4 +-- 6 files changed, 29 insertions(+), 23 deletions(-) rename filter_skeleton.cc => deprecated/filter_skeleton.cc (100%) diff --git a/GPSBabel.pro b/GPSBabel.pro index f6c74520b..e6036e37e 100644 --- a/GPSBabel.pro +++ b/GPSBabel.pro @@ -94,6 +94,7 @@ HEADERS = \ csv_util.h \ defs.h \ explorist_ini.h \ + filter.h \ filterdefs.h \ garmin_device_xml.h \ garmin_fs.h \ diff --git a/filter_skeleton.cc b/deprecated/filter_skeleton.cc similarity index 100% rename from filter_skeleton.cc rename to deprecated/filter_skeleton.cc diff --git a/filter.h b/filter.h index 20c31d591..6d83e10a5 100644 --- a/filter.h +++ b/filter.h @@ -25,6 +25,19 @@ class Filter { public: + Filter() = default; + // Provide virtual public destructor to avoid undefined behavior when + // an object of derived class type is deleted through a pointer to + // its base class type. + // https://wiki.sei.cmu.edu/confluence/display/cplusplus/OOP52-CPP.+Do+not+delete+a+polymorphic+object+without+a+virtual+destructor + virtual ~Filter() = default; + // And that requires us to explictly default the move and copy operations. + // https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#c21-if-you-define-or-delete-any-default-operation-define-or-delete-them-all. + Filter(const Filter&) = default; + Filter& operator=(const Filter&) = default; + Filter(Filter&&) = default; + Filter& operator=(Filter&&) = default; + virtual arglist_t* get_args() = 0; virtual void init() diff --git a/route.cc b/route.cc index cc440fa84..21c7d1067 100644 --- a/route.cc +++ b/route.cc @@ -254,7 +254,7 @@ track_del_wpt(route_head* rte, Waypoint* wpt) } void -route_disp(const route_head* rh, std::nullptr_t /* wc */) +route_disp(const route_head* /* rh */, std::nullptr_t /* wc */) { // wc == nullptr } diff --git a/trackfilter.cc b/trackfilter.cc index df783cd7f..a10432700 100644 --- a/trackfilter.cc +++ b/trackfilter.cc @@ -29,6 +29,7 @@ #include "xmlgeneric.h" #include #include +#include #include #include /* for snprintf */ #include /* for qsort */ @@ -555,13 +556,12 @@ void TrackFilter::trackfilter_split() int new_track_flag; if ((opt_interval == 0) && (opt_distance == 0)) { - struct tm t1, t2; // FIXME: This whole function needs to be reconsidered for arbitrary time. time_t tt1 = buff[i]->GetCreationTime().toTime_t(); time_t tt2 = buff[j]->GetCreationTime().toTime_t(); - t1 = *localtime(&tt1); - t2 = *localtime(&tt2); + const struct tm t1 = *localtime(&tt1); + const struct tm t2 = *localtime(&tt2); new_track_flag = ((t1.tm_year != t2.tm_year) || (t1.tm_mon != t2.tm_mon) || (t1.tm_mday != t2.tm_mday)); @@ -917,9 +917,9 @@ TrackFilter::faketime_t TrackFilter::trackfilter_faketime_check(const char* time char c; const char* cin; struct tm time; - int timeparse = 1; + bool timeparse = true; faketime_t result; - result.force = 0; + result.force = false; i = j = 0; strncpy(fmtstart, "00000101000000", sizeof(fmtstart)); @@ -928,7 +928,7 @@ TrackFilter::faketime_t TrackFilter::trackfilter_faketime_check(const char* time while ((c = *cin++)) { if (c=='f') { - result.force = 1; + result.force = true; continue; } @@ -939,7 +939,7 @@ TrackFilter::faketime_t TrackFilter::trackfilter_faketime_check(const char* time if (timeparse) { if (c == '+') { fmtstart[i++] = '\0'; - timeparse = 0; + timeparse = false; } else { if (fmtstart[i] == '\0') { fatal(MYNAME "-faketime: parameter too long \"%s\"!\n", timestr); @@ -965,33 +965,25 @@ TrackFilter::faketime_t TrackFilter::trackfilter_faketime_check(const char* time return result; } -int TrackFilter::trackfilter_faketime() /* returns number of track points left after filtering */ +void TrackFilter::trackfilter_faketime() { - faketime_t faketime; - queue* elem, *tmp; - int i, dropped, inside = 0; - - if (opt_faketime != nullptr) { - faketime = trackfilter_faketime_check(opt_faketime); - } - dropped = inside = 0; + assert(opt_faketime != nullptr); + faketime_t faketime = trackfilter_faketime_check(opt_faketime); - for (i = 0; i < track_ct; i++) { + for (int i = 0; i < track_ct; i++) { route_head* track = track_list[i].track; QUEUE_FOR_EACH((queue*)&track->waypoint_list, elem, tmp) { Waypoint* wpt = (Waypoint*)elem; - if (opt_faketime != nullptr && (!wpt->creation_time.isValid() || faketime.force)) { + if (!wpt->creation_time.isValid() || faketime.force) { wpt->creation_time = QDateTime::fromTime_t(faketime.start); faketime.start += faketime.step; } } } - - return track_pts - dropped; } int TrackFilter::trackfilter_points_are_same(const Waypoint* wpta, const Waypoint* wptb) diff --git a/trackfilter.h b/trackfilter.h index 99a6c88b3..7c0ca79e7 100644 --- a/trackfilter.h +++ b/trackfilter.h @@ -216,11 +216,11 @@ private: typedef struct faketime_s { time_t start; int step; - int force; + bool force; } faketime_t; faketime_t trackfilter_faketime_check(const char* timestr); - int trackfilter_faketime(); /* returns number of track points left after filtering */ + void trackfilter_faketime(); /* returns number of track points left after filtering */ int trackfilter_points_are_same(const Waypoint* wpta, const Waypoint* wptb); void trackfilter_segment_head(const route_head* rte); -- 2.30.2